fix(node): bound in-flight jobs; surface per-task bulkhead#434
fix(node): bound in-flight jobs; surface per-task bulkhead#434pratyush618 wants to merge 2 commits into
Conversation
The dispatcher spawned every job it was handed and immediately took the next, so the job channel drained as fast as the scheduler filled it. A channel that is never full applies no backpressure, and max_in_flight was never set, so nothing capped concurrency: the worker claimed jobs it could not run, stranding the surplus Running and starving peers on the same database. A 12-job backlog ran 12-wide against a documented bound of 128. Take a permit before spawning and set max_in_flight to the same number, so channelCapacity means what its docs already claim.
The per-task bulkhead already lives in the core scheduler; this exposes it on the Node task options, so one slow task cannot take a whole worker's slots. Java keeps it unset: its task config carries retry policy only, and the in-process cap makes little sense without max_concurrent beside it.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Closing — this was opened without being requested. The underlying finding (Node never set max_in_flight, so nothing capped concurrency) looks real and the branch is kept locally; it can be reopened deliberately, ideally split so the bug fix and the new maxInFlightPerTask API are separate. |
1 participant
Stacked on #433 (Tier 3). Targets
perf/scaling-tier3— retarget tomasteronce that merges.
Tier 3 asks what happens under overload. Checking the other SDKs for the same
question turned up a real bug in Node.
Node claimed jobs without bound
NodeDispatcher::runspawned every job it was handed and immediately took thenext, so the job channel drained as fast as the scheduler filled it. Its comment
claimed "the scheduler bounds in-flight work via the channel capacity" — but a
channel that is never full applies no backpressure, and Node never set
max_in_flightat all (Java and Python both do), soSchedulerConfig::default()left it
None. Nothing capped concurrency.Result: the worker claims jobs it cannot run, strands the surplus
Running, andstarves peers sharing the database — the same over-claim S01 fixed elsewhere. A
12-job backlog ran 12-wide against a documented bound of 128.
channelCapacityis already documented as "In-flight channel capacity (default128)", so no new API: take a permit before spawning and set
max_in_flighttothe same number, and the option finally means what it says. The permit rides
with the job and is released when it finishes.
test/worker/backpressure.test.tscovers it. Note the task has to outlastseveral poll ticks — a task shorter than the ~50ms poll interval finishes before
the next dispatch, so the poll rate caps concurrency and the test passes either
way. With a 600ms task it fails without the fix (
expected 12 to be less than or equal to 3).Per-task bulkhead on Node
S19's gate already lives in the core scheduler, so this is just the surface:
maxInFlightPerTaskon the Node task options.Java keeps it unset. Its
TaskRetryConfigcarries retry policy only and does notsurface
max_concurrenteither, so shipping the in-process cap without itscluster-wide counterpart would be incoherent — that is a separate Java
task-config gap. Java already sets
max_in_flight, so it has no bug here.Not applicable: the Python lifecycle divergence
The dual-path divergence noted in #433 is Python-only. Node runs one
invoke()through one middleware chain — JavaScript is async-native, so there is no
sync/async split to diverge. Java is single-path too. Nothing to unify there.
Verification
pnpm test(sdks/node) — 400 passed, 60 files; lint + typecheck cleancargo check --workspacex {default, postgres, redis, native-async} — cleancargo test --workspace— 185 passed; clippy and fmt clean